Skip to content

Conversation

@BrandonStalnaker
Copy link
Collaborator

Background

  • Originally we had the idea to do an extension to provide a kind of automatic functionality but unfortunately that isn't possible for reasons shared bellow. Alternatively I created a subclass of SceneDelegate that customers could use as a subclass to their SceneDelegate and call super on the important methods. I'm not sure this is very useful though. To me it seems easier to just call the manual methods but I wan't to put this PR up to prompt the discussion.

The Problem with Protocol Extensions

// This WON'T work as expected
extension UIWindowSceneDelegate {
func scene(_ scene: UIScene, openURLContexts URLContexts: Set) {
for context in URLContexts {
MParticle.sharedInstance().handleURLContext(context)
}
}
}
Why it fails:

UIKit uses Objective-C runtime to check if the delegate responds to selectors

Protocol extension methods aren't added to the class's method table - they exist only at the Swift protocol level

When UIKit calls scene(_:openURLContexts:), it does [delegate respondsToSelector:@selector(scene:openURLContexts:)] which returns NO for protocol extension methods

What Has Changed

  • Implement MPSceneDelegate which implements the UIWindowSceneDelegate protocol with what we'd recommend customers call. They can use this as the subclass of their SceneDelegate and have automatic implementations assuming they don't have any SceneDelegate implementation of their own.

Checklist

  • I have performed a self-review of my own code.
  • I have made corresponding changes to the documentation.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have tested this locally.

Reference Issue (For employees only. Ignore if you are an outside contributor)

@BrandonStalnaker BrandonStalnaker self-assigned this Dec 3, 2025
@BrandonStalnaker BrandonStalnaker requested a review from a team as a code owner December 3, 2025 22:21
@BrandonStalnaker BrandonStalnaker changed the title feat/SDKE-644-Implement-MPSceneDelegate feat: Implement-MPSceneDelegate Dec 4, 2025
@BrandonStalnaker BrandonStalnaker changed the title feat: Implement-MPSceneDelegate feat: Implement MPSceneDelegate Dec 4, 2025
@BrandonStalnaker BrandonStalnaker force-pushed the feat/SDKE-644-Implement-Protocol-Extension branch 2 times, most recently from 232e115 to 06edcf6 Compare December 4, 2025 15:11
@rmi22186
Copy link
Member

rmi22186 commented Dec 4, 2025

Not sure if this PR was done since you said "up for discussion" in it, but can you add tests?

@BrandonStalnaker
Copy link
Collaborator Author

BrandonStalnaker commented Dec 4, 2025

I'm adding the tests now but, as I discuss in the background section above, I'd like to use this review for us to decide if we should even commit this code. The original intent was was to do an extension but when I implemented and tested I found that it didn't work so I attempted this subclassing idea. It works but I don't know if it provides much value to customers so I'm not sure it should be merged. Thoughts?

@BrandonStalnaker BrandonStalnaker force-pushed the feat/SDKE-644-Implement-Protocol-Extension branch from 06edcf6 to 23f2cdb Compare December 4, 2025 17:42
@jamesnrokt
Copy link
Collaborator

I don't know if it provides much value to customers?

Having seen the implementation I think you're right. In the sync meeting about this project we said things should be opt in and this feels like it breaks that by lumping multiple features into a single delegate.

I think this is something we can solve for with docs

@denischilik
Copy link
Contributor

I'm okay with having MPSceneDelegate — it may be useful for some customers. At the same time, we should allow clients to perform the integration themselves and explain that option clearly in the documentation.

However, right now we have actual logic implemented inside MPSceneDelegate.
My question is: do we want this logic to be owned and maintained by the SDK, or do we expect customers to re-implement it themselves when not using the base delegate?

@rmi22186
Copy link
Member

rmi22186 commented Dec 4, 2025

I'm all for less code - if customers request then we implement in the future.

@BrandonStalnaker
Copy link
Collaborator Author

Closing for now. Can re-open if needed by customers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants